Introduce ddsrt_abort and ddsrt_exit - #1877
Conversation
This will consolidate abort calls. This will make it easy to override, change, or suppress the abort feature. Borrowing a bit of structure from Rust, the new `ddsrt_abort` was placed inside ddsrt::process.
eboasson
left a comment
There was a problem hiding this comment.
Thanks @poetinger, I do see the practical value of not calling abort() directly but putting something in between if one is using an embedded platform — at least I have found something of the sort useful in the past 🙂.
So that's the good bit.
The bad bit is that not all the aborts in the core code have been replaced, for example src/core/cdr/src/dds_cdrstream_keys.part.c have a bunch left.
Not replacing abort in all locations (like tests) is a defensible choice, but I think it only makes life more complicated to sometimes use abort and sometimes use ddsrt_abort: now one has to decide which one to use. So I think all should be replaced and only a single call to abort should remain, in ddsrt_abort.
The subject of exit is another story. Cyclone DDS comes in multiple parts: the core bit is a library—this should never terminate and so should never call exit to begin with, and it doesn't—and then there are some tools/examples/tests that use it. I don't quite see why something like ddsperf can't just call exit.
So unless you have a good argument why exit must be avoided at all cost, I'd take out the 11 changes related to it.
|
Sounds reasonable to me. I'll remove |
Signed-off-by: Philip Oetinger <philip.oetinger@zettascale.tech>
From feedback comment, it really is unnecessary. Signed-off-by: Philip Oetinger <philip.oetinger@zettascale.tech>
743e592 to
a86760a
Compare
This pull request:
ddsrt_abortandddsrt_exitabortwithddsrt_abortinddsc,ddsrt,cdr, andddsi